Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve slime connection on Windows #385

Merged
merged 6 commits into from
Jun 5, 2019
Merged

Conversation

Hamayama
Copy link
Collaborator

Windows 上 で M-x slime を実行すると、固まったり、
応答が表示されなかったり、終了できなくなったりする件を、改善してみました。

  • message-waiting-p ( modes/lisp-mode/swank-protocol.lisp ):
    Windows 版の sbcl では、
    usocket:wait-for-input を呼び出す前にイベントオブジェクトをリセットしないと、
    2回目以降の usocket:wait-for-input で、ウェイトしなくなってしまう。
    このため、WSAResetEvent でイベントオブジェクトをリセットするようにした。
    (本現象は、usocket:wait-for-input の引数にソケットを1個だけ指定したときに発生する。
    ソケットが1個だけのときは、内部情報 (wait-list) を毎回作らないで使いまわすようになっており、
    イベントオブジェクトも使いまわしになるためリセットが必要だった。。。
    https://github.com/usocket/usocket/blob/2b92b917cf9bd763492461b5b9814e801d1b063f/usocket.lisp#L348 )

  • port-available-p ( modes/lisp-mode/util.lisp ):
    usocket:socket-listen のところで、
    :reuse-address t を指定しているが、
    複数プロセスが同一ポートで待ち受けして変になるケースがあったため、
    :reuse-address nil に変更した。

  • start-thread ( modes/lisp-mode/lisp-mode.lisp ):

    • 前述の usocket:wait-for-input の対応の結果、
      Reduce cpu usage in lisp-mode on Windows #378 の (sleep 0.001) は不要になったため削除した。
    • change-connection 例外がいろいろな場所で発生することがあったため、
      handler-case の範囲を広げた。
    • message-waiting-p のタイムアウト値を 10 秒から 1 秒にへらした。
      (終了時に最大10秒固まったため)
  • add-hook *exit-editor-hook* ( modes/lisp-mode/lisp-mode.lisp ):

    • ソケットクローズしないと終了しないことがあったため、
      ソケットクローズを追加した。

@@ -1254,10 +1252,15 @@
'highlight-evaluation-region)

;; workaround for windows
;; (quit slime and close sockets to exit lem normally)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

関数を定義してコメントではなくdocstringに書くと良いと思います。

@Hamayama
Copy link
Collaborator Author

終了処理を見直ししました。docstring も書いてみました。
(なんでソケットをクローズすると終了するのか、いまいち分かっていませんが。。。)
ただ、まだ、たまに終了できないことがあるようです。
(端末ごと落とせば終了します)

usocket の件は、一応報告してみました。
usocket/usocket#50

@Hamayama
Copy link
Collaborator Author

どうもソケットをクローズすると、まれに以下のエラーが出ることがあったため、
ソケットのクローズは止めました。

;; swank:close-connection: end of file on #<SB-SYS:FD-STREAM for "socket 127.0.0.1:64348, peer: 127.0.0.1:54601" {100487ADC3}>
;; swank:close-connection: end of file on #<SB-SYS:FD-STREAM for "socket 127.0.0.1:64348, peer: 127.0.0.1:54601" {100487ADC3}>

終了できないことがある件については、また別の機会に調べてみます。。。

@snmsts
Copy link
Member

snmsts commented May 30, 2019

swank:close-connectionについてはちょっと待ってあげないといけない気はしますね。

@Hamayama
Copy link
Collaborator Author

うーむ。sleep を入れるとエラーは出なくなりましたが、
やはり終了するときとしないときがあります。
今は、ソケットのクローズをしてもしなくても、ほとんど変化がないようです。
(昨日は終了しやすくなったと思ったのですが。。。)

そもそも (uiop:quit) を送っているので、
ソケットを切らなくても終了するはずなのですが。。。

@Hamayama
Copy link
Collaborator Author

Hamayama commented Jun 3, 2019

いくつか改善をしました。

  1. REPL上で応答が表示されないことがある件
    → LAN アナライザ (WiresharkPortable + npcap) で見てみると、
    swank-server からの応答が 'Thread not found: :REPL-THREAD' となっている
    → server の起動と slime-connect の間に sleep を入れたら改善した
    (接続は成功するがコマンドを処理できない時間帯があるのかも)

  2. lem を終了できない件
    → LAN アナライザ (WiresharkPortable + npcap) で見てみると、
    usocket:socket-close しても FIN が出ない
    → sbcl 内部の sockint::shutdown + sockint::close に切り換えた
    → FIN は出るようになった。
    少しよくなったが、まだ終了しないことがある。。。

  3. swank-protocol.lisp
    → 不要な定義を削除した

@Hamayama
Copy link
Collaborator Author

Hamayama commented Jun 4, 2019

swank server との接続の setup のところで、
応答の :return コマンドとは別に :indentation-update というコマンドが来ており、
送受信のシーケンスがずれておかしくなっていたため、
:return コマンドだけを待つようにしました。

これによって、swank-repl モジュールのロード完了を、正常に待てるようになったため、
前回追加した sleep は削除しました。

(lem を終了できないことがある件は、そのままです。。。)

@snmsts snmsts merged commit 472f308 into lem-project:master Jun 5, 2019
@Hamayama Hamayama deleted the lispconn branch June 13, 2019 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants